Skip to content

Conversation

@rmccorm4
Copy link
Contributor

@rmccorm4 rmccorm4 commented Nov 8, 2025

Overview:

  • Skips broken etcd_ha tests to unblock other unrelated PRs failing CI due to it
  • Proper fix will be coming soon and should revert this change when ready

Details:

Minimally attempting to unblock the following PRs, but new and rebased PRs will also be affected:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Summary by CodeRabbit

Release Notes

  • Tests
    • Updated fault-tolerance test suite to temporarily skip several ETCD-related tests pending fixes.

Note: No user-facing features or changes in this release.

@rmccorm4 rmccorm4 requested review from a team as code owners November 8, 2025 00:26
@github-actions github-actions bot added the ci Issues/PRs that reference CI build/test label Nov 8, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 8, 2025

Walkthrough

These changes add pytest skip decorators to multiple test functions across three ETCD HA fault-tolerance test files, marking them as broken and temporarily disabling them from execution without modifying their internal logic.

Changes

Cohort / File(s) Summary
ETCD HA Fault-Tolerance Tests—Skip Decorators
tests/fault_tolerance/etcd_ha/test_sglang.py, tests/fault_tolerance/etcd_ha/test_trtllm.py, tests/fault_tolerance/etcd_ha/test_vllm.py
Added @pytest.mark.skip(reason="Broken, temporarily disabled") decorators to 14 test functions (4 in test_sglang.py, 6 in test_trtllm.py, 4 in test_vllm.py). No changes to test signatures or internal logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • All changes are identical, repetitive skip decorator additions
  • No logic alterations or functional modifications
  • Homogeneous pattern across all three files minimizes reasoning overhead

Poem

🐰 Some tests take a hop, skip, and a bounds,
Temporarily paused without a sound,
Fourteen tests now rest with grace,
Till bugs are fixed—they'll win the race! ✨

Pre-merge checks

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive PR description covers overview and details adequately, but leaves template sections incomplete and issue reference unresolved. Specify which GitHub issue this closes (replace #xxx), and provide more detail on which tests are affected and why they're broken for reviewer clarity.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: skipping broken etcd_ha tests to unblock CI for unrelated PRs.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rmccorm4 rmccorm4 enabled auto-merge (squash) November 8, 2025 01:04
@rmccorm4 rmccorm4 merged commit 560bb2f into main Nov 8, 2025
30 checks passed
@rmccorm4 rmccorm4 deleted the rmccormick/skip_broken_etcd_ha_test branch November 8, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci Issues/PRs that reference CI build/test size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants